Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(autocomplete): scroll to top of the list each time dropdown open #11578

Merged
Splaktar merged 1 commit into from
Mar 5, 2020
Merged

fix(autocomplete): scroll to top of the list each time dropdown open #11578

Splaktar merged 1 commit into from
Mar 5, 2020

Conversation

@marosoft
Copy link
Contributor

@marosoft marosoft commented Jan 7, 2019 •
edited
Loading

At the moment the drop down stays at the same position.

Fixes #10479

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

After reopening the drop down of the autocomplete the scroll starts from the same position when it was closed.
Issue Number: #10479

What is the new behavior?

After reopening the drop down the scroll starts from the top.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Jan 7, 2019
@Splaktar Splaktar self-requested a review January 7, 2019 20:47
@Splaktar Splaktar self-assigned this Jan 7, 2019
@Splaktar Splaktar added needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved P5: nice to have These issues will not be fixed without community contributions. labels Jan 22, 2019
@Splaktar Splaktar added this to the 1.1.13 milestone Jan 22, 2019
Splaktar
Splaktar suggested changes Jan 22, 2019
View reviewed changes
Copy link
Contributor

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please try to reproduce this issue in a test and provide the test in this PR?

@Splaktar Splaktar added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs needs: unit tests This PR needs unit tests to cover the changes being proposed labels Jan 22, 2019
@Splaktar Splaktar modified the milestones: 1.1.13, 1.1.14 Feb 10, 2019
@Splaktar Splaktar modified the milestones: 1.1.14, 1.1.15 Mar 14, 2019
@Splaktar Splaktar modified the milestones: 1.1.15, 1.1.16, 1.1.18, 1.1.19 Mar 29, 2019
@Splaktar Splaktar modified the milestones: 1.1.19, 1.1.20 May 30, 2019
@Splaktar Splaktar modified the milestones: 1.1.20, 1.1.21 Aug 15, 2019
@Splaktar Splaktar removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Sep 30, 2019
@Splaktar
Copy link
Contributor

Splaktar commented Sep 30, 2019

@marosoft can you please rebase this?

@Splaktar Splaktar modified the milestones: 1.1.21, 1.1.22 Oct 22, 2019
@marosoft marosoft force-pushed the wip/autocomplete-scroll branch from 22afd97 to 662719f Compare November 2, 2019 21:58
@marosoft
Copy link
Contributor Author

marosoft commented Nov 2, 2019 •
edited
Loading

@marosoft can you please rebase this?

@Splaktar I just rebased and refactored my fix. Sorry for a delay here.
As there was a bigger change introduced (having standard and virtual modes) I had to find a better solution as my original fix was working for the virtual list only.

CodePen - demo of both modes (copied from the official docs - autocomplete demo) including my latest fix.

@marosoft marosoft force-pushed the wip/autocomplete-scroll branch from 662719f to 33a0668 Compare November 2, 2019 22:52
@Splaktar Splaktar added needs: manual testing This issue or PR needs to have some manual testing and verification done and removed needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved labels Nov 3, 2019
@marosoft marosoft force-pushed the wip/autocomplete-scroll branch from 33a0668 to 8fbc895 Compare November 3, 2019 18:45
@marosoft
Copy link
Contributor Author

marosoft commented Nov 3, 2019

@Splaktar I added a test for it. Please let me know if it looks fine.

Splaktar
Splaktar approved these changes Nov 3, 2019
View reviewed changes
Copy link
Contributor

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the minor test name tweak, this looks great, thank you!

@Splaktar Splaktar added pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review type: bug and removed needs: manual testing This issue or PR needs to have some manual testing and verification done needs: unit tests This PR needs unit tests to cover the changes being proposed labels Nov 3, 2019
@marosoft
fix(autocomplete): scroll to top of the list each time dropdown open
58fd670 
At the moment the drop down stays at the same position.

Fixes angular#10479
@marosoft marosoft force-pushed the wip/autocomplete-scroll branch from 8fbc895 to 58fd670 Compare November 3, 2019 19:56
@Splaktar Splaktar requested a review from mmalerba March 4, 2020 19:15
mmalerba
mmalerba approved these changes Mar 4, 2020
View reviewed changes
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Splaktar Splaktar merged commit 498c9ed into angular:master Mar 5, 2020
@marosoft marosoft deleted the wip/autocomplete-scroll branch March 22, 2020 19:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@Splaktar Splaktar Splaktar approved these changes

@mmalerba mmalerba mmalerba approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Splaktar Splaktar

Labels

cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P5: nice to have These issues will not be fixed without community contributions. pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review type: bug

Projects

None yet

Milestone

1.1.22

Development

Successfully merging this pull request may close these issues.

autocomplete: scroll doesn't return to top when dropdown is opened again

4 participants

@marosoft @Splaktar @mmalerba @googlebot